It will conflict with postgres/postgres@0f65f3e#diff-358df0f060121a25b138de1f148f4d3db078ba7f1aa46897f4dbbad4be724150

Also cannot understand how it could overflow here since we calculate norm_query_buflen as

	/*
	 * Allow for $n symbols to be longer than the constants they replace.
	 * Constants must take at least one byte in text form, while a $n symbol
	 * certainly isn't more than 11 bytes, even if n reaches INT_MAX.  We
	 * could refine that limit based on the max value of n for the current
	 * query, but it hardly seems worth any extra effort to do so.
	 */
	norm_query_buflen = query_len + jstate->clocations_count * 10;

Thanks for the detailed explanation. I agree that, given the existing sizing rule:

norm_query_buflen = query_len + jstate->clocations_count * 10

and the invariant that each constant contributes at least one byte, while each generated placeholder is bounded to at most 11 bytes, the original overflow claim is not supported. I’ll withdraw the “critical security vulnerability” framing.

The snprintf change is only a defensive hardening/style change, not a demonstrated bug fix. I also see your point that this may conflict with the upstream PostgreSQL pg_stat_statements normalisation changes, so this PR probably is not the right vehicle.

I’m happy to close this, or retitle it as a very small defensive cleanup if the project would still value replacing sprintf with bounded formatting. Otherwise, I’ll defer to the upstream PostgreSQL normalisation work.

Thanks for the detailed explanation. I agree that, given the existing sizing rule:

norm_query_buflen = query_len + jstate->clocations_count * 10

and the invariant that each constant contributes at least one byte, while each generated placeholder is bounded to at most 11 bytes, the original overflow claim is not supported. I’ll withdraw the “critical security vulnerability” framing.

The snprintf change is only a defensive hardening/style change, not a demonstrated bug fix. I also see your point that this may conflict with the upstream PostgreSQL pg_stat_statements normalisation changes, so this PR probably is not the right vehicle.

I’m happy to close this, or retitle it as a very small defensive cleanup if the project would still value replacing sprintf with bounded formatting. Otherwise, I’ll defer to the upstream PostgreSQL normalisation work.

I will suggest sending a fix to the postgres community. I searched the pgsql-hackers mailing list and did not find any patches for using snprintf in pg_stats_statements. It would be really useful to discuss the issue with the postgres community as well.

Also, we are based on the postgres kernel and constantly update it (since postgres is constantly developing). Therefore, the main way to fix something in the kernel code is to get a fix from postgres's code (most likely it has already been fixed), or fix something in postgres and then get a patch back. Otherwise, it is quite easy to forget to add a patch during the rebasing process.

Sometimes, an issue cannot be fixed in postgres. Well, that's why our project exists. Let's do it in Cloudberry.

But it seems to me that your patch is more important for postgres than for Cloudberry. Let us fix it in postgres! And sooner or later, it will be fixed in all projects based on postgres.

If you suddenly have any problems with the patch review, let us know, some of the project participants are also active contributors to postgres.